ck - mintRollovers should remove rollovers after successful execution

[sherlock-admin]

ck

medium

mintRollovers should remove rollovers after successful execution

Summary

mintRollovers should remove rollovers after successful execution

Vulnerability Detail

After a successful rollover, the mintRollovers just replaces the assets and epochId of a user with new values.

                    rolloverQueue[index].assets = assetsToMint;
                    rolloverQueue[index].epochId = _epochId;
                    // only pay relayer for successful mints
                    executions++;

The issue is that after a successful rollover that a user had enrolled for has succeeded, they remain in the queue with new values and their old index.

Let's say they want to enroll some more assets. Since they still exist in the rolloverQueue, their index in the queue will remain the same:

        // check if user has already queued up a rollover
        if (ownerToRollOverQueueIndex[_receiver] != 0) {
            // if so, update the queue
            uint256 index = getRolloverIndex(_receiver);
            rolloverQueue[index].assets = _assets;
            rolloverQueue[index].epochId = _epochId;

Now going back to the mintRollovers function, their index has already been recorded as successful:

if (executions > 0) rolloverAccounting[_epochId] = index;

This means, that when the starting index of the mintRollovers function will be a value higher than the user's position in the queue and no rollover will happen for their additional assets.

uint256 index = rolloverAccounting[_epochId];

Impact

Users who enlist for rollover of additional assets will not have it executed.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L361-L459

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Carousel/Carousel.sol#L253-L257

Tool used

Manual Review

Recommendation

Remove a user from the rolloverQueue after a successful rollover so that when they enlist again, they are pushed to the end of the queue.

[iamckn]

Escalate for 10 USDC

This should not have been excluded as it breaks rolling functionality and prevents users from rolling over additional assets.

[sherlock-admin]

Escalate for 10 USDC

This should not have been excluded as it breaks rolling functionality and prevents users from rolling over additional assets.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[hrishibhat]

Escalation rejected

Valid informational
A user cannot enter the same epoch mint queue twice is a design limitation, not a valid high/medium

[sherlock-admin]

Escalation rejected

Valid informational
A user cannot enter the same epoch mint queue twice is a design limitation, not a valid high/medium

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.