evan - User will almost always incur a loss when they rollover

[sherlock-admin]

evan

high

User will almost always incur a loss when they rollover

Summary

The rollover mechanism doesn't take into account the amount of fund won from the previous epoch. This means that users will almost always incur a loss when they enlist in a rollover.

Vulnerability Detail

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L408-L412
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L436
Observe that it doesn't matter how much the user has won in the epoch, queue[index].assets of the old epoch token will be burned, and queue[index].assets - relayerFee of the new epoch token will be minted.

Impact

Assume the user has deposited 100 units of assets for an old epoch x, claimTVL[x] / finalTVL[x] = 1.5 (which is a fairly realistic number), and a new epoch y has now been created.

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L366
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L164
If the user manually withdraws from epoch x, they will get 150 units of assets. They can then deposit for epoch y, getting roughly (deposit fee) 150 shares.

However, if the user enlisted all 100 shares of epoch x for rollover, then they will only get roughly (relayer fee) 100 shares of epoch y.

See the POC for more detail.

Code Snippet

Please add the following test to CarouselTest.t.sol and run it by itself forge test -m testRolloverLoss
I believe the claimTVL[x] / finalTVL[x] is 10 here. A bit unrealistic but demonstrates the point better. The user gets more than 9 times more when they withdraw & deposit instead of rolling over (See the comparison at the end).

    function testRolloverLoss() public {
        // pretend it's deposit & stuff for other epochs & users
        // also ensures there's enough balance later on in the test
        deal(UNDERLYING, address(vault), 1000 ether, true);

        // ------------- Situation 1: user uses rollover -------------------

        // create first epoch
        uint40 _epochBegin = uint40(block.timestamp + 1 days);
        uint40 _epochEnd = uint40(block.timestamp + 2 days);
        uint256 _epochId = 2;
        uint256 _emissions = 10 ether;

        deal(emissionsToken, address(vault), 10 ether, true);
        vault.setEpoch(_epochBegin, _epochEnd, _epochId);
        vault.setEmissions( _epochId, _emissions);
    
        // deposit in first epoch
        helperDepositInEpochs(_epochId,USER, false);

        // enlist in rollover for next epoch
        vm.startPrank(USER);
        vault.enlistInRollover(_epochId, vault.balanceOf(USER, _epochId), USER);
        vm.stopPrank();

        // resolve first epoch
        vm.warp(_epochEnd + 1 days);
        vm.startPrank(controller);
        vault.resolveEpoch(_epochId);
        vm.stopPrank();

        // create second epoch
        _epochBegin = uint40(block.timestamp + 1 days);
        _epochEnd = uint40(block.timestamp + 2 days);
        _epochId = 3;
        _emissions = 10 ether;

        deal(emissionsToken, address(vault), 10 ether, true);
        vault.setEpoch(_epochBegin, _epochEnd, _epochId);
        vault.setEmissions( _epochId, _emissions);

        // simulate prev epoch win
        stdstore
            .target(address(vault))
            .sig("claimTVL(uint256)")
            .with_key(2)
            .checked_write(100 ether);

        // let relayer rollover for user
        vm.startPrank(relayer);
        vault.mintRollovers(_epochId, 1);
        vm.stopPrank();

        assertEq(vault.balanceOf(USER, 2), 0);
        uint256 new_epoch_balance = vault.balanceOf(USER, 3);

        // ------------- Situation 2: user doesn't use rollover -------------------
        // exact setup as above

        // create first epoch
        _epochBegin = uint40(block.timestamp + 1 days);
        _epochEnd = uint40(block.timestamp + 2 days);
        _epochId = 4;
        _emissions = 10 ether;

        deal(emissionsToken, address(vault), 10 ether, true);
        vault.setEpoch(_epochBegin, _epochEnd, _epochId);
        vault.setEmissions( _epochId, _emissions);
    
        // deposit in first epoch
        helperDepositInEpochs(_epochId,USER, false);

        // resolve first epoch
        vm.warp(_epochEnd + 1 days);
        vm.startPrank(controller);
        vault.resolveEpoch(_epochId);
        vm.stopPrank();

        // create second epoch
        _epochBegin = uint40(block.timestamp + 1 days);
        _epochEnd = uint40(block.timestamp + 2 days);
        _epochId = 5;
        _emissions = 10 ether;

        deal(emissionsToken, address(vault), 10 ether, true);
        vault.setEpoch(_epochBegin, _epochEnd, _epochId);
        vault.setEmissions( _epochId, _emissions);

        // simulate prev epoch win
        stdstore
            .target(address(vault))
            .sig("claimTVL(uint256)")
            .with_key(4)
            .checked_write(100 ether);

        // manually withdraw & deposit into the new
        vm.startPrank(USER);
        uint256 withdrawn = vault.withdraw(4, vault.balanceOf(USER, 4), USER, USER);
        IERC20(UNDERLYING).approve(address(vault), withdrawn);
        vault.deposit(5, withdrawn, USER);
        vm.stopPrank();

        assertEq(vault.balanceOf(USER, 4), 0);
        uint256 new_epoch_balance2 = vault.balanceOf(USER, 5);

        // ------------- Compare results -------------------
        // both situations the user lost all of their old epoch tokens
        // but the user got 9 times more new epoch tokens when they don't rollover
        assertTrue(new_epoch_balance2 > new_epoch_balance * 9);
    }

Tool used

Manual Review
Foundry

Recommendation

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L436
I believe entitledShares should be used here instead of queue[index].assets

Duplicate of #163